feat(runtime): expose process lifecycle hooks to embedding hosts - #514
feat(runtime): expose process lifecycle hooks to embedding hosts#514MertBasar0 wants to merge 3 commits into
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. |
|
Codex review: needs maintainer review before merge. Reviewed August 25, 2026, 6:07 AM ET / 10:07 UTC. ClawSweeper reviewWhat this changesAdds optional lifecycle hooks for embedding hosts to observe or admit ACP agent process spawning, failures, and exits with session or probe correlation. Merge readinessKeep open for explicit maintainer approval of a new public runtime contract: the implementation and proof are solid, but awaited host admission hooks intentionally make host callback behavior part of ACP startup availability. Priority: P2 Review scores
Verification
Live VerificationCommand: Result: PASS (completed) Assertions:
How this fits togetherEmbedding hosts use the runtime API to create ACP clients and sessions, which launch an external ACP agent process. This change exposes the launch boundary to the host before startup and reports its eventual failure or exit. flowchart LR
A[Embedding host] --> B[ACP runtime]
B --> C[Lifecycle-aware client startup]
C --> D[Admission hooks]
D --> E[ACP agent process]
E --> F[ACP session initialization]
E --> G[Failure and exit observation]
Decision needed
Why: VISION.md treats public conventions as durable API surface; this optional hook is intentionally small and backend-oriented, but its ordering and failure semantics cannot safely be chosen by automated review. Before merge
Agent review detailsSecurityNone. Review metrics
Merge-risk optionsMaintainer options:
Technical reviewBest possible solution: Land the narrow lifecycle seam only if maintainers want host-owned admission to be a stable runtime API and explicitly accept that hosts own its timeout and recovery policy. Do we have a high-confidence way to reproduce the issue? Not applicable as a bug reproduction: this is a new API surface, and the PR supplies a real embedding-host trace for the proposed behavior. Is this the best way to solve the issue? Unclear: the small host-owned seam fits the backend direction, but maintainers must choose whether its awaited admission behavior is the permanent API contract. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against d4c16ab32154. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rank-up movesOptional improvements that raise the rating; they are not merge blockers.
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (7 earlier review cycles)
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@steipete This is ready for a maintainer product/API decision. The current head includes real embedding-host proof, all repository checks pass, and ClawSweeper reports no actionable findings. Could you confirm whether ACPX should adopt the opt-in awaited onBeforeSpawn / onSpawned admission hooks as a public runtime contract, with timeout and recovery policy owned by embedding hosts? If so, the PR is ready as-is. If you prefer bounded timeout/cancellation or nonblocking semantics, I’m happy to adjust. |
Related: #499
Context: openclaw/openclaw#128006
What Problem This Solves
Embedding hosts can launch ACP agents through
AcpClientorAcpRuntime, but they currently have no supported way to durably record process ownership around the exact spawn boundary. Hosts that need crash recovery or process-lease reconciliation must either wrap ACPX internals or accept a gap between preparing a launch and learning the child PID.Why This Change Was Made
This adds an optional lifecycle contract with immutable, correlated events for pre-spawn admission, successful spawn, spawn failure, and process exit. Runtime-created clients also receive stable session/probe scopes.
The two admission hooks are deliberately awaited: rejecting before spawn prevents launch, while rejecting after spawn terminates the child before returning the error. Failure and exit observers remain best-effort and cannot mask the underlying process outcome. Exit notification is ordered after spawned admission settles, avoiding a stale-lease race when a child exits quickly.
This is an embedding seam only. It does not add persistence, process-tree cleanup, or host-specific lease policy to ACPX, and therefore does not claim to close #499.
User Impact
Embedding developers can now implement durable host-owned launch tracking without wrapping ACPX spawn internals. Existing clients are unaffected because every lifecycle hook is optional.
Evidence
pnpm run checkpasses in a clean Linux checkout: formatting, TypeScript checks, type-aware lint, package and replay-viewer builds, 942 repository tests, and 130 coverage tests.onSpawnFailednonblocking while preserving synchronous invocation, correlation data, and logging for synchronous throws and asynchronous rejections.@agentclientprotocol/codex-acp1.6.2 adapter passes; redacted runtime proof is included below.openclaw/acpxmainat commit time.Real embedding-host proof
Environment: WSL2 Linux x64, Node v24.15.0, PR head
08a8601, published@agentclientprotocol/codex-acp1.6.2, existing Codex ChatGPT login. No credential values were logged. The temporary external host imported the built publicdist/runtime.jsAPI and usedcreateAcpRuntime,createRuntimeStore, andcreateAgentRegistry; it was not added to the PR.The first scenario initialized a real ACP session through
codex-acp, then closed it. The second used a real Node child that exited with code 17 while a slow but successfulonSpawnedadmission hook was pending. The third used a real OS spawn failure whileonSpawnFaileddeliberately returned a Promise that never settles.The successful adapter PID and the admission-test PID were both confirmed exited after the run. No model prompt or inference was needed; this proof exercises the embedding boundary, real adapter process lifecycle, ACP session initialization, correlation, the repaired startup-exit path, and the repaired spawn-failure semantics.
AI assistance
The implementation was AI-assisted. I reviewed the resulting lifecycle contract, failure semantics, tests, generated diff, and real embedding-host trace, and validated it with the checks listed above.